Skip to content

Conversation

@encounter
Copy link
Contributor

The majority of changes are removing reliance on std::io. Instead of using write!, we can use the infallible Vec::extend.

This removed most of the error handling from serialize, with a couple of exceptions:

  • The ctor/dtor errors "If there's a ctor/dtor, there should be another name in this sequence". I replaced these with writing [invalid] to the output.
  • The conversion from Vec to String. I modified this to just emit lossy UTF-8 if there are any errors. (Though I don't expect this to ever happen in practice, given that parse should have handled any invalid UTF-8 already)

With those refactored, serialize now returns a String instead of Result<String>. This also allowed me to remove the Io error variant entirely. #![no_std] success!

Also included:

  • Bump to Rust edition 2021
  • Some minor clippy suggestions
  • Version bump to 0.11, given the serialize return type is an API breaking change

@mstange
Copy link
Owner

mstange commented Feb 5, 2025

Looks good to me except for the addition of the format! calls. I think those are new memory allocations that we didn't have with write!. And all of them are just for surrounding a value with fixed strings. Could you replace each format! call with a sequence of three calls to extend?

@encounter
Copy link
Contributor Author

It looks like all of the format! calls are actually formatting integers, so I'm not sure of a way to avoid format! or .to_string() on those.

@mstange
Copy link
Owner

mstange commented Feb 6, 2025

Oh I missed that.

I think one option would be to use the itoa crate for that. It seems to be a very lightweight dependency. What do you think?

@encounter
Copy link
Contributor Author

itoa looks nice! I’ll add that in.

@encounter
Copy link
Contributor Author

encounter commented Feb 6, 2025

All set, I replaced integer formatting with itoa, and single-character extends with push.

Also, I was able to avoid allocations for nested serialize calls by simply calling self.serialize(val) rather than self.w.extend(serialize(val).as_bytes()).

@mstange
Copy link
Owner

mstange commented Feb 6, 2025

Excellent, thank you!

@mstange mstange merged commit 9bc5f3a into mstange:main Feb 6, 2025
1 check passed
@mstange
Copy link
Owner

mstange commented Feb 6, 2025

Released as 0.11.0.

@mstange
Copy link
Owner

mstange commented Feb 6, 2025

Out of curiosity, what's your use case for running this in a no_std context?

@encounter
Copy link
Contributor Author

For my objdiff tool, I've been working on ensuring all dependencies are #![no_std] for the core layer that can compile to WASM. Specifically for reduced binary size and control over WASI imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants